Skip to content

Conversation

@onur-ozkan
Copy link
Contributor

@onur-ozkan onur-ozkan commented Nov 17, 2025

Description

This change improves peer removal during heartbeat by switching from a two-pass remove logic to an in-place retain with very small size intermediate variable removed_peers_count.

Change checklist

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • A changelog entry has been made in the appropriate crates

@onur-ozkan onur-ozkan changed the title gossipsub: in-place negative-score peer removal refactor(gossipsub): in-place negative-score peer removal Nov 17, 2025
elenaf9
elenaf9 previously approved these changes Nov 18, 2025
Copy link
Member

@elenaf9 elenaf9 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

@onur-ozkan onur-ozkan force-pushed the opt-peer-removal-iter branch from ceec87e to 797293e Compare November 18, 2025 17:37
@mergify
Copy link
Contributor

mergify bot commented Nov 18, 2025

This pull request has merge conflicts. Could you please resolve them @onur-ozkan? 🙏

Copy link
Member

@jxs jxs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, and thanks for looking into it! Since extract_if has been stabilized I think we can use it to make it easier to count the removed peers wdyt?

@onur-ozkan
Copy link
Contributor Author

Hi, and thanks for looking into it! Since extract_if has been stabilized I think we can use it to make it easier to count the removed peers wdyt?

We would need to raise MSRV to very recent version. I am fine with that but I think there will be many downstream users not liking this.

This change improves peer removal during `heartbeat` by switching
from a two-pass remove logic to an in-place `retain` with very small
size intermediate variable `removed_peers_count`.

Signed-off-by: Onur Özkan <[email protected]>
Signed-off-by: Onur Özkan <[email protected]>
@onur-ozkan onur-ozkan force-pushed the opt-peer-removal-iter branch from 797293e to 44cb2c5 Compare November 19, 2025 15:17
Copy link
Member

@jxs jxs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, and thanks for looking into it! Since extract_if has been stabilized I think we can use it to make it easier to count the removed peers wdyt?

We would need to raise MSRV to very recent version. I am fine with that but I think there will be many downstream users not liking this.

sorry for the delay.
Ah right it was only stabilized in October, ok let's leave a TODO then for when MSRV is updated to support it.
Thanks!

@onur-ozkan
Copy link
Contributor Author

Hi, and thanks for looking into it! Since extract_if has been stabilized I think we can use it to make it easier to count the removed peers wdyt?

We would need to raise MSRV to very recent version. I am fine with that but I think there will be many downstream users not liking this.

sorry for the delay. Ah right it was only stabilized in October, ok let's leave a TODO then for when MSRV is updated to support it. Thanks!

Done :)

Copy link
Member

@jxs jxs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@jxs
Copy link
Member

jxs commented Dec 6, 2025

ah, fmt job seems to be failing

Signed-off-by: Onur Özkan <[email protected]>
@onur-ozkan onur-ozkan force-pushed the opt-peer-removal-iter branch from 37f29a3 to adce9f3 Compare December 6, 2025 06:59
Copy link
Member

@jxs jxs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks!

@jxs jxs added the send-it label Dec 7, 2025
@mergify mergify bot dismissed elenaf9’s stale review December 7, 2025 11:13

Approvals have been dismissed because the PR was updated after the send-it label was applied.

@mergify
Copy link
Contributor

mergify bot commented Dec 7, 2025

Merge Queue Status

✅ The pull request has been merged

This pull request spent 6 seconds in the queue, with no time running CI.
The checks were run in-place.

Required conditions to merge
  • #approved-reviews-by >= 1 [🛡 GitHub branch protection]
  • #changes-requested-reviews-by = 0 [🛡 GitHub branch protection]
  • #review-threads-unresolved = 0 [🛡 GitHub branch protection]
  • any of [🛡 GitHub branch protection]:
    • check-success = Check rustdoc intra-doc links
    • check-neutral = Check rustdoc intra-doc links
    • check-skipped = Check rustdoc intra-doc links
  • any of [🛡 GitHub branch protection]:
    • check-success = rustfmt
    • check-neutral = rustfmt
    • check-skipped = rustfmt
  • any of [🛡 GitHub branch protection]:
    • check-success = Test libp2p-metrics
    • check-neutral = Test libp2p-metrics
    • check-skipped = Test libp2p-metrics
  • any of [🛡 GitHub branch protection]:
    • check-success = Compile on x86_64-apple-darwin
    • check-neutral = Compile on x86_64-apple-darwin
    • check-skipped = Compile on x86_64-apple-darwin
  • any of [🛡 GitHub branch protection]:
    • check-success = Compile on x86_64-pc-windows-msvc
    • check-neutral = Compile on x86_64-pc-windows-msvc
    • check-skipped = Compile on x86_64-pc-windows-msvc
  • any of [🛡 GitHub branch protection]:
    • check-success = Test libp2p-yamux
    • check-neutral = Test libp2p-yamux
    • check-skipped = Test libp2p-yamux
  • any of [🛡 GitHub branch protection]:
    • check-success = Test libp2p-swarm
    • check-neutral = Test libp2p-swarm
    • check-skipped = Test libp2p-swarm
  • any of [🛡 GitHub branch protection]:
    • check-success = Test libp2p-mdns
    • check-neutral = Test libp2p-mdns
    • check-skipped = Test libp2p-mdns
  • any of [🛡 GitHub branch protection]:
    • check-success = Test libp2p-relay
    • check-neutral = Test libp2p-relay
    • check-skipped = Test libp2p-relay
  • any of [🛡 GitHub branch protection]:
    • check-success = Test libp2p-uds
    • check-neutral = Test libp2p-uds
    • check-skipped = Test libp2p-uds
  • any of [🛡 GitHub branch protection]:
    • check-success = Test libp2p-rendezvous
    • check-neutral = Test libp2p-rendezvous
    • check-skipped = Test libp2p-rendezvous
  • any of [🛡 GitHub branch protection]:
    • check-success = Test libp2p
    • check-neutral = Test libp2p
    • check-skipped = Test libp2p
  • any of [🛡 GitHub branch protection]:
    • check-success = Test libp2p-noise
    • check-neutral = Test libp2p-noise
    • check-skipped = Test libp2p-noise
  • any of [🛡 GitHub branch protection]:
    • check-success = Test libp2p-gossipsub
    • check-neutral = Test libp2p-gossipsub
    • check-skipped = Test libp2p-gossipsub
  • any of [🛡 GitHub branch protection]:
    • check-success = Test rw-stream-sink
    • check-neutral = Test rw-stream-sink
    • check-skipped = Test rw-stream-sink
  • any of [🛡 GitHub branch protection]:
    • check-success = Test libp2p-dns
    • check-neutral = Test libp2p-dns
    • check-skipped = Test libp2p-dns
  • any of [🛡 GitHub branch protection]:
    • check-success = Test libp2p-floodsub
    • check-neutral = Test libp2p-floodsub
    • check-skipped = Test libp2p-floodsub
  • any of [🛡 GitHub branch protection]:
    • check-success = Test libp2p-swarm-derive
    • check-neutral = Test libp2p-swarm-derive
    • check-skipped = Test libp2p-swarm-derive
  • any of [🛡 GitHub branch protection]:
    • check-success = Test libp2p-pnet
    • check-neutral = Test libp2p-pnet
    • check-skipped = Test libp2p-pnet
  • any of [🛡 GitHub branch protection]:
    • check-success = Test libp2p-tls
    • check-neutral = Test libp2p-tls
    • check-skipped = Test libp2p-tls
  • any of [🛡 GitHub branch protection]:
    • check-success = Test multistream-select
    • check-neutral = Test multistream-select
    • check-skipped = Test multistream-select
  • any of [🛡 GitHub branch protection]:
    • check-success = Test libp2p-kad
    • check-neutral = Test libp2p-kad
    • check-skipped = Test libp2p-kad
  • any of [🛡 GitHub branch protection]:
    • check-success = Test libp2p-core
    • check-neutral = Test libp2p-core
    • check-skipped = Test libp2p-core
  • any of [🛡 GitHub branch protection]:
    • check-success = Test libp2p-tcp
    • check-neutral = Test libp2p-tcp
    • check-skipped = Test libp2p-tcp
  • any of [🛡 GitHub branch protection]:
    • check-success = Test libp2p-websocket
    • check-neutral = Test libp2p-websocket
    • check-skipped = Test libp2p-websocket
  • any of [🛡 GitHub branch protection]:
    • check-success = IPFS Integration tests
    • check-neutral = IPFS Integration tests
    • check-skipped = IPFS Integration tests
  • any of [🛡 GitHub branch protection]:
    • check-success = Test libp2p-identify
    • check-neutral = Test libp2p-identify
    • check-skipped = Test libp2p-identify
  • any of [🛡 GitHub branch protection]:
    • check-success = Test libp2p-dcutr
    • check-neutral = Test libp2p-dcutr
    • check-skipped = Test libp2p-dcutr
  • any of [🛡 GitHub branch protection]:
    • check-success = Compile on wasm32-unknown-emscripten
    • check-neutral = Compile on wasm32-unknown-emscripten
    • check-skipped = Compile on wasm32-unknown-emscripten
  • any of [🛡 GitHub branch protection]:
    • check-success = Compile with select features (mdns tcp dns tokio)
    • check-neutral = Compile with select features (mdns tcp dns tokio)
    • check-skipped = Compile with select features (mdns tcp dns tokio)
  • any of [🛡 GitHub branch protection]:
    • check-success = Test libp2p-request-response
    • check-neutral = Test libp2p-request-response
    • check-skipped = Test libp2p-request-response
  • any of [🛡 GitHub branch protection]:
    • check-success = Compile on wasm32-unknown-unknown
    • check-neutral = Compile on wasm32-unknown-unknown
    • check-skipped = Compile on wasm32-unknown-unknown
  • any of [🛡 GitHub branch protection]:
    • check-success = Test libp2p-mplex
    • check-neutral = Test libp2p-mplex
    • check-skipped = Test libp2p-mplex
  • any of [🛡 GitHub branch protection]:
    • check-success = Test libp2p-ping
    • check-neutral = Test libp2p-ping
    • check-skipped = Test libp2p-ping
  • any of [🛡 GitHub branch protection]:
    • check-success = Test libp2p-plaintext
    • check-neutral = Test libp2p-plaintext
    • check-skipped = Test libp2p-plaintext
  • any of [🛡 GitHub branch protection]:
    • check-success = Test libp2p-autonat
    • check-neutral = Test libp2p-autonat
    • check-skipped = Test libp2p-autonat
  • any of [🛡 GitHub branch protection]:
    • check-success = Test libp2p-quic
    • check-neutral = Test libp2p-quic
    • check-skipped = Test libp2p-quic
  • any of [🛡 GitHub branch protection]:
    • check-success = Test libp2p-webrtc
    • check-neutral = Test libp2p-webrtc
    • check-skipped = Test libp2p-webrtc
  • any of [🛡 GitHub branch protection]:
    • check-success = Test quick-protobuf-codec
    • check-neutral = Test quick-protobuf-codec
    • check-skipped = Test quick-protobuf-codec
  • any of [🛡 GitHub branch protection]:
    • check-success = Test libp2p-identity
    • check-neutral = Test libp2p-identity
    • check-skipped = Test libp2p-identity
  • any of [🛡 GitHub branch protection]:
    • check-success = Test libp2p-allow-block-list
    • check-neutral = Test libp2p-allow-block-list
    • check-skipped = Test libp2p-allow-block-list
  • any of [🛡 GitHub branch protection]:
    • check-success = Test libp2p-swarm-test
    • check-neutral = Test libp2p-swarm-test
    • check-skipped = Test libp2p-swarm-test
  • any of [🛡 GitHub branch protection]:
    • check-success = Check for changes in proto files
    • check-neutral = Check for changes in proto files
    • check-skipped = Check for changes in proto files
  • any of [🛡 GitHub branch protection]:
    • check-success = Ensure that Cargo.lock is up-to-date
    • check-neutral = Ensure that Cargo.lock is up-to-date
    • check-skipped = Ensure that Cargo.lock is up-to-date
  • any of [🛡 GitHub branch protection]:
    • check-success = Test libp2p-connection-limits
    • check-neutral = Test libp2p-connection-limits
    • check-skipped = Test libp2p-connection-limits
  • any of [🛡 GitHub branch protection]:
    • check-success = examples
    • check-neutral = examples
    • check-skipped = examples
  • any of [🛡 GitHub branch protection]:
    • check-success = Run all WASM tests
    • check-neutral = Run all WASM tests
    • check-skipped = Run all WASM tests
  • any of [🛡 GitHub branch protection]:
    • check-success = Test libp2p-memory-connection-limits
    • check-neutral = Test libp2p-memory-connection-limits
    • check-skipped = Test libp2p-memory-connection-limits
  • any of [🛡 GitHub branch protection]:
    • check-success = Test libp2p-webtransport-websys
    • check-neutral = Test libp2p-webtransport-websys
    • check-skipped = Test libp2p-webtransport-websys
  • any of [🛡 GitHub branch protection]:
    • check-success = Test libp2p-perf
    • check-neutral = Test libp2p-perf
    • check-skipped = Test libp2p-perf
  • any of [🛡 GitHub branch protection]:
    • check-success = Compile with MSRV
    • check-neutral = Compile with MSRV
    • check-skipped = Compile with MSRV
  • any of [🛡 GitHub branch protection]:
    • check-success = manifest_lint
    • check-neutral = manifest_lint
    • check-skipped = manifest_lint
  • any of [🛡 GitHub branch protection]:
    • check-success = Test libp2p-server
    • check-neutral = Test libp2p-server
    • check-skipped = Test libp2p-server
  • any of [🛡 GitHub branch protection]:
    • check-success = Compile on wasm32-wasip1
    • check-neutral = Compile on wasm32-wasip1
    • check-skipped = Compile on wasm32-wasip1

@mergify mergify bot added the queued label Dec 7, 2025
@mergify mergify bot merged commit b0edc68 into libp2p:master Dec 7, 2025
68 of 70 checks passed
@mergify mergify bot removed the queued label Dec 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants